Skip to content

Fix async pattern in RenderAsync#429

Merged
sebastienros merged 1 commit intomainfrom
sebros/async
Dec 14, 2021
Merged

Fix async pattern in RenderAsync#429
sebastienros merged 1 commit intomainfrom
sebros/async

Conversation

@sebastienros
Copy link
Copy Markdown
Owner

Fixes #428

@sebastienros sebastienros requested a review from lahma December 14, 2021 19:33
Copy link
Copy Markdown
Collaborator

@lahma lahma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, pretty much the same where to I reduced the code to get it working.

context.EnterChildScope();

try
{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will be returns if an exception is occured?!!

Copy link
Copy Markdown
Owner Author

@sebastienros sebastienros Dec 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An Exception

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this intentionally?!! coz isn't throw before

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we need a remark on that method to make clear to everyone

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the behavior has changed here. New code just awaits to prevent control flow from getting out with unfinished continuations that could run after finally has been processed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What confuses me the old behavior returns the actual string from the StringBuilderPool instace in both case (Exception or Without Exception) but now the string is returned if no exception occurs. Am I right?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code would have done the await on Awaited method which would have bubbled the exception. So both versions will throw, this one obeys template context life time.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong, if there had been an exception, it would have been thrown by the task that is returned.
No exception is caught here, so everything will bubble up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FluidTemplateExtensions.RenderAsync premature release scope

3 participants